Update types for h11 v0.14#579
Merged
lovelydinosaur merged 20 commits intoencode:masterfrom Sep 27, 2022
Merged
Conversation
Allow for h11 V. 013. Facilitates installation alongside latest version of Uvicorn.
This also reverted commit f9a9847 which was dropped during rebase with the `master` branch. We probably do not want to suggest that passing `None` to the `write` function is proper. The short circuit there (return on empty buffer) should be targeted to an empty byte array.
Contributor
Author
|
Let me know if you'd like me to rebase to clean this history up. For now, I've just extended the commits from the original branch for clarity. |
0.12 does not define `h11.Event`
Contributor
Not needed, no. We have "squash and merge" enforced on this project, so pull requests always end up as our atomic unit of history. |
lovelydinosaur
requested changes
Sep 26, 2022
Contributor
lovelydinosaur
left a comment
lovelydinosaur
left a comment
There was a problem hiding this comment.
A linting fix here...
lovelydinosaur
approved these changes
Sep 27, 2022
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Extends and supersedes #526.
Updates the types in
_send_eventandrecieve_eventfor compatibility with h11 0.14.0.Tests and
mypywere run locally on h11 0.13.0 and 0.14.0.Closes #503
Closes #498
Unblocks #509
Differences from #526
Removes cast from the "hot loop"
h11 0.14.0 includes the upstream fix python-hyper/h11@04cc0f7 allowing us to remove the cast and
isinstancecheck from the hot loop which @graingert expressed concerns about in #526 (comment). A cast is still required for the return value as mypy is not type-narrowing on theisclause. We'd probably have to do a weirdisinstance/issubclasscombination to avoid the cast, which would bring us back to the original concerns about affected performance.Interestingly,
mypypasses with h11 0.13.0 so we may have been able to use this implementation without the upstream change.Addresses narrower types
Updates the
_send_eventtypes to be narrower as mentioned in #526 (comment). If overloads are added to the h11 library, this will allow us to remove an unnecessary null check as noted in #526 (comment).Bumps minimum h11 version
Bumps the minimum h11 version to 0.13.0 which is required for the
h11.Eventtype. I imagine we could create a conditional stub for the type if support for older versions of h11 is important.